feat(middleware): define HTTP response pre-return interface - #3073

gator-agent

PR Review Status

The interface and caller migration are internally consistent, and the pre-0.1.0 symbol changes are not blockers. One enforcement gap must be fixed before this can advance: OpenShell currently accepts the new response binding even though no supported HTTP relay invokes it.

Action required: keep HTTP_RESPONSE/PRE_RETURN manifest registration rejected until the response runner is wired into every supported HTTP relay, or land that wiring with this change.

Blocking findings:

• GATOR-11ef5b93-01: accepted response middleware bindings are silently unenforced

Carried findings:

• None

gator-agent

PR Review Status

Thanks @pimlock. I checked your September 2 update that rejects HTTP_RESPONSE/PRE_RETURN until relay dispatch exists and adds a regression test. The manifest now fails closed for that binding, so GATOR-11ef5b93-01 is resolved; the independent delta review found no new blocking issues. Branch Checks are running, so Gator will monitor the pipeline next.

Blocking findings:

• No blocking findings remain

Carried findings:

• GATOR-11ef5b93-01: resolved at this head by rejecting the undispatched response binding

Review feedback

I do not think the V1 interface is ready to approve yet. I found four contract-level issues:

1. The PR omits response trailers required by accepted issue #2691. The stream exposes only preflight, body, and session-end events, so middleware cannot inspect or safely mutate trailers. This leaves trailer-carried integrity metadata or sensitive data outside response enforcement. Please restore trailer events and declared trailer names, or formally re-scope the accepted issue.

   Event definitions · Accepted issue

2. block_delivery expands middleware authority beyond the accepted design. Issue #2691 explicitly defers successful response denial, but this PR adds it to preflight and body processing regardless of on_error. It also describes only a “platform error,” without specifying its status, body, HEAD behavior, or stable error code. Please remove it from V1 or update and re-accept the design with complete client-visible semantics.

   Preflight blocking · Body blocking

3. A shared body-mode list conflicts with stage-local payload limits. max_payload_bytes is the effective limit for the current stage, while every stage is documented as receiving the same permitted_body_modes. For a 2 MiB response and stages limited to 1 MiB and 4 MiB, WHOLE_BODY_BYTES must either be incorrectly offered to the first stage or incorrectly withheld from the second. Please compute response-shape eligibility once, then intersect it with each stage effective limit.

   Preflight limits and modes

4. The disconnect enum change is parse-compatible but not semantically wire-compatible. Previously, value 2 meant generic PEER_DISCONNECT; it now means specifically DOWNSTREAM_DISCONNECT. A new middleware connected to an older supervisor will classify an old upstream disconnect as downstream. Please preserve value 2 as direction-neutral and allocate new values for directional reasons, or version the contract.

   New session-end enum

The overall stream and transport shape is otherwise coherent. I also verified that the earlier Gator finding is resolved: HTTP_RESPONSE/PRE_RETURN registration is now rejected until relay dispatch lands.

Additional review feedback

A follow-up contract pass found three additional items beyond my earlier review:

1. The PR description and protobuf disagree about reserved trailer fields. The description says response trailers are deferred from V1 “with field numbers reserved,” but proto/supervisor_middleware.proto contains no reserved declaration. The current event/result oneofs immediately assign session-end field 3 and expose no reservation for the trailer messages described in #2691. Please either add the intended reservations or correct the description and document how trailers will be added compatibly.

   Current event/result envelopes · Accepted trailer design

2. Several other V1 contract changes have not been reconciled with the accepted plan. In addition to trailers and blocking, this interface replaces the planned body_end event with end_of_stream, adds skip_remaining and cross-unit deferral, reduces stream inputs from the full effective limit to half the limit, adds a whole-body accumulation timeout, and moves diagnostics into the decision envelopes. Some may be good changes, but they alter the accepted public contract. Please update #2691 and obtain maintainer confirmation for the revised V1 surface before freezing its field numbers and semantics.

   end_of_stream · skip_remaining · deferral contract

3. The new upstream-disconnect path lacks behavioral coverage. The mapping unit test verifies that WebSocketTerminationCause::UpstreamDisconnect maps to the enum, but no relay test verifies that an upstream EOF actually delivers UPSTREAM_DISCONNECT to an opened middleware session. Since the directional split changes externally observed lifecycle semantics, please add a relay-level test that closes the upstream and asserts the terminal reason.

   Upstream relay EOF handling · Mapping-only test